[SPARK-56911][SQL] Simplify Cast to decimal codegen under ANSI mode by gengliangwang · Pull Request #55936 · apache/spark

gengliangwang · 2026-05-17T23:30:20Z

What changes were proposed in this pull request?

Extend CastUtils.java with two helpers for decimal precision adjustment and use them from Cast.changePrecision (both eval and codegen). The new helpers mutate the input Decimal in place (matching the in-place semantics of the existing inline codegen), so they're safe to call on the temporary produced by Decimal.fromString(...) / Decimal.apply(...) / decimal-arithmetic results.

Helpers added:

changePrecisionExact(Decimal, int, int, QueryContext): ANSI throw on overflow, preserves the per-call-site QueryContext so error messages keep their query-origin info.
changePrecisionOrNull(Decimal, int, int): non-ANSI, returns null on overflow (no QueryContext needed).

Cast.scala changes:

changePrecision eval method dispatches on nullOnOverflow and delegates to the appropriate helper.
changePrecision codegen method has three branches now: the existing canNullSafeCast fast path (unchanged), a nullOnOverflow branch (inline), and the ANSI throw branch which now emits a one-line CastUtils.changePrecisionExact(...) call instead of the 5-line if/else overflow block.

Why are the changes needed?

Part of SPARK-56908 (umbrella). The ANSI throw branch of Cast.changePrecision is hit by every cast to decimal that may overflow (very common in TPC-DS, where cast(int as decimal(7,2)) is widespread). Collapsing the 5-line inline body to one line shrinks the generated Java source for those plans.

Does this PR introduce any user-facing change?

No. The compiled behavior is identical; only the emitted Java source text changes.

How was this patch tested?

build/sbt "catalyst/testOnly *CastSuite *CastWithAnsiOnSuite *CastWithAnsiOffSuite *AnsiCastSuite *TryCastSuite *DecimalSuite"

332/332 pass.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Cursor 1.x

gengliangwang · 2026-05-17T23:32:30Z

Stack overview (SPARK-56908 umbrella)

This PR is part of the SPARK-56908 codegen-simplification series. Current status:

Merged:

[SPARK-56909][SQL] Simplify Cast to int/long codegen under ANSI mode #55934 — [SPARK-56909][SQL] Simplify Cast to int/long codegen under ANSI mode
[SPARK-56913][SQL] Simplify BinaryArithmetic byte/short codegen under ANSI mode #55938 — [SPARK-56913][SQL] Simplify BinaryArithmetic byte/short codegen under ANSI mode
[SPARK-56915][SQL] Simplify MakeDate/MakeInterval codegen under ANSI mode #55940 — [SPARK-56915][SQL] Simplify MakeDate/MakeInterval codegen under ANSI mode
[SPARK-56916][SQL] Simplify ElementAt array codegen under ANSI mode #55941 — [SPARK-56916][SQL] Simplify ElementAt array codegen under ANSI mode
[SPARK-56935][SQL] Simplify GetArrayItem codegen and consolidate ElementAtUtils into ArrayExpressionUtils #55973 — [SPARK-56935][SQL] Simplify GetArrayItem codegen and consolidate ElementAtUtils into ArrayExpressionUtils

Open:

[SPARK-56910][SQL] Simplify Cast to byte/short codegen under ANSI mode #55935 — [SPARK-56910][SQL] Simplify Cast to byte/short codegen under ANSI mode (branches off master)
[SPARK-56911][SQL] Simplify Cast to decimal codegen under ANSI mode #55936 — [SPARK-56911][SQL] Simplify Cast to decimal codegen under ANSI mode (stacked on [SPARK-56910][SQL] Simplify Cast to byte/short codegen under ANSI mode #55935)
[SPARK-56912][SQL] Simplify Cast to boolean codegen under ANSI mode #55937 — [SPARK-56912][SQL] Simplify Cast to boolean codegen under ANSI mode (stacked on [SPARK-56911][SQL] Simplify Cast to decimal codegen under ANSI mode #55936)
[SPARK-56914][SQL] Simplify decimal arithmetic codegen under ANSI mode #55939 — [SPARK-56914][SQL] Simplify decimal arithmetic codegen under ANSI mode (stacked on [SPARK-56911][SQL] Simplify Cast to decimal codegen under ANSI mode #55936)
[SPARK-57004][SQL] Simplify CheckOverflowInSum codegen under ANSI mode #56063 — [SPARK-57004][SQL] Simplify CheckOverflowInSum codegen under ANSI mode (independent)

gengliangwang · 2026-05-21T18:32:32Z

Audited this PR for the same lessons surfaced by @viirya and @cloud-fan on #55938 (and applied to #55934 / #55939):

Are the helpers redundant with an existing Scala API? CastUtils.changePrecisionExact calls d.changePrecision(precision, scale) in-place; Decimal.toPrecision(...) does the same logical work but clones the input first (val copy = clone()). The in-place variant avoids a per-row allocation, so the helpers serve a distinct purpose.
Are the eval-path additions redundant? The pre-PR changePrecision method was an 8-line if-changePrecision { ... } else { if nullOnOverflow null else throw } block; the helpers shrink it to 5 lines. Not a one-liner that was already concise.

So no changes needed on this PR for that review.

Extend `CastUtils.java` with two helpers for decimal precision adjustment and use them from `Cast.changePrecision` (both the eval and codegen implementations). The new helpers mutate the input `Decimal` in place (matching the behavior of the existing inline codegen), so they're safe to call on the temporary produced by `Decimal.fromString(...)` / `Decimal.apply(...)` / decimal-arithmetic results. Helpers added: * `changePrecisionExact(Decimal, int, int, QueryContext)`: ANSI throw on overflow, preserves the per-call-site `QueryContext` so error messages keep their query-origin info. * `changePrecisionOrNull(Decimal, int, int)`: non-ANSI, returns `null` on overflow (no `QueryContext` needed). `Cast.scala` changes: * `changePrecision` eval method dispatches on `nullOnOverflow` and delegates to the appropriate helper. * `changePrecision` codegen method has three branches now: the existing `canNullSafeCast` fast path (unchanged), a `nullOnOverflow` branch (inline), and the ANSI throw branch which now emits a one-line `CastUtils.changePrecisionExact(...)` call instead of the 5-line `if/else` overflow block. Part of SPARK-56908 (umbrella). The ANSI throw branch of `Cast.changePrecision` is hit by every cast to decimal that may overflow (very common in TPC-DS, where `cast(int as decimal(7,2))` is widespread). Collapsing the 5-line inline body to one line shrinks the generated Java source for those plans. No. The compiled behavior is identical; only the emitted Java source text changes. ``` build/sbt "catalyst/testOnly *CastSuite *CastWithAnsiOnSuite \ *CastWithAnsiOffSuite *AnsiCastSuite *TryCastSuite *DecimalSuite \ *ExpressionClassIdentitySuite" ``` 337/337 pass. Generated-by: Cursor 1.x

cloud-fan

Small, well-scoped refactor in the SPARK-56908 codegen-simplification series. The two new CastUtils helpers (changePrecisionExact / changePrecisionOrNull) wrap Decimal.changePrecision with throw/null overflow handling and are reused from both the eval path and the ANSI-throw codegen branch.

The conversation comment already audited the redundancy questions raised on #55938 (vs. Decimal.toPrecision, vs. one-liner eval path) and the answers hold up: toPrecision clones the input, the helpers mutate in place; the old eval block was 8 lines, not 1. Semantics check out — when Decimal.changePrecision returns false the input is left untouched (early returns in Decimal.changePrecision precede the field writes), so the error message still cites the original value, matching pre-PR behavior.

Findings below are all minor / housekeeping: stale class-level Javadoc, time-bound phrasing in a new section comment, and a now-redundant argument that the refactor naturally invites cleaning up.

cloud-fan · 2026-05-27T19:18:32Z

    throw QueryExecutionErrors.castingCauseOverflowError(v, DOUBLE, SHORT);
  }
+
+  // ----- decimal precision adjustment -----


The class Javadoc above scopes this file as "ANSI overflow-checked narrowing to byte / short" and explicitly says "The helpers below cover byte / short only". That's no longer accurate once this section is added — the file now also hosts decimal precision adjustment, which is neither narrowing nor byte/short. Worth widening the header doc (e.g., drop the "byte/short only" claim and frame it as "ANSI overflow-checked operations used from Cast.doGenCode / eval paths"), or this PR's helpers will read as "shouldn't be here".

Done — class Javadoc rewritten to drop the byte/short-only scope (now "ANSI overflow-checked casts" generally) and moved the byte/short narrowing limitation into an inline comment above the integral-narrowing section.

cloud-fan · 2026-05-27T19:18:32Z

+  // Mutates the input Decimal in place to apply the target precision/scale on
+  // the per-row hot path.
+


"Used by Cast.changePrecision (and by BinaryArithmetic / DivModLike in follow-up PRs)" describes the rollout, not the helpers themselves. Once the follow-up PRs land (or don't) the phrasing is wrong or misleading, and the caller enumeration drifts as the codebase evolves. The two facts a future reader actually needs are already in the first and last sentences: in-place mutation, and the hot-path rationale. Suggest dropping the middle:

Suggested change

// Mutates the input Decimal in place to apply the target precision/scale on

// the per-row hot path.

// Mutates the input Decimal in place to avoid the per-row clone() done by

// Decimal.toPrecision, since these helpers are called on the per-row hot path.

Adopted your suggestion. Also added a follow-up sentence (per @viirya's third note) pinning the invariant that Decimal.changePrecision returns false before any write-back, so d stays in its pre-cast state and the error message cites the original value.

cloud-fan · 2026-05-27T19:18:33Z

         |}
       """.stripMargin
+    } else {
+      val errorContextCode = getContextOrNullCode(ctx, !nullOnOverflow)


Pre-refactor this lived in a shared else block that handled both nullOnOverflow == true and nullOnOverflow == false, so !nullOnOverflow was load-bearing. Now this line is reached only inside the else branch where nullOnOverflow == false, so !nullOnOverflow is always true — which is the default of getContextOrNullCode. Can drop the second arg:

Suggested change

val errorContextCode = getContextOrNullCode(ctx, !nullOnOverflow)

val errorContextCode = getContextOrNullCode(ctx)

Done — dropped to getContextOrNullCode(ctx). You're right, with the else if (nullOnOverflow) branch split out, !nullOnOverflow is always true here, which is the default.

viirya · 2026-05-27T19:32:55Z

Thanks for the cleanup — refactor is semantically equivalent and the codegen shrink on the ANSI throw branch is a nice win. A few non-blocking nits:

The CastUtils class-level Javadoc still says the helpers are for "ANSI overflow-checked narrowing to byte / short", and the second paragraph reinforces the byte/short-only scope. After this PR that's no longer accurate. Consider rewording the lead sentence to something more general (e.g. "Static helpers used by Cast.doGenCode (and corresponding eval paths) for ANSI overflow-checked casts.") and moving the byte/short limitation note into an inline comment above the integral-narrowing section.
In the new ANSI codegen branch:
```
val errorContextCode = getContextOrNullCode(ctx, !nullOnOverflow)
```
Now that the else if (nullOnOverflow) branch is split out, we're guaranteed nullOnOverflow == false here, so !nullOnOverflow is always true — i.e. the default. Simplifying to getContextOrNullCode(ctx) matches the other ANSI sites in this file (e.g. lines 1483, 1520, 1603).
The // Mutates the input Decimal in place ... comment above the helpers captures the key invariant. It might be worth adding one more sentence: on overflow, Decimal.changePrecision returns false before writing back any of decimalVal / longVal / _precision / _scale, so the input d is still in its original externally-visible state when changePrecisionExact throws. The error message correctly reflects the pre-cast value because of that — worth pinning down so a future change to Decimal.changePrecision doesn't quietly break it.

Otherwise LGTM.

…ecisionExact invariant, drop redundant getContextOrNullCode arg

gengliangwang · 2026-05-27T19:51:58Z

@viirya thanks — all three addressed in edf8784b2e6:

Widened the CastUtils class Javadoc (dropped the byte/short-only scope, moved the byte/short limitation into an inline comment above the integral-narrowing section).
Dropped the redundant !nullOnOverflow arg → getContextOrNullCode(ctx), matching the other ANSI call sites at lines 1483 / 1520 / 1603.
Added the invariant note about Decimal.changePrecision returning false before writing back, so the input d stays in its pre-cast state when changePrecisionExact throws — pinned so a future change to Decimal.changePrecision doesn't silently break the error-message contract.

lint-scala clean, 332/332 Cast + Decimal tests pass.

gengliangwang · 2026-05-28T04:53:09Z

@cloud-fan @viirya thanks for the review. Merging to master/4.3

### What changes were proposed in this pull request? Extend `CastUtils.java` with two helpers for decimal precision adjustment and use them from `Cast.changePrecision` (both eval and codegen). The new helpers mutate the input `Decimal` in place (matching the in-place semantics of the existing inline codegen), so they're safe to call on the temporary produced by `Decimal.fromString(...)` / `Decimal.apply(...)` / decimal-arithmetic results. Helpers added: * `changePrecisionExact(Decimal, int, int, QueryContext)`: ANSI throw on overflow, preserves the per-call-site `QueryContext` so error messages keep their query-origin info. * `changePrecisionOrNull(Decimal, int, int)`: non-ANSI, returns `null` on overflow (no `QueryContext` needed). `Cast.scala` changes: * `changePrecision` eval method dispatches on `nullOnOverflow` and delegates to the appropriate helper. * `changePrecision` codegen method has three branches now: the existing `canNullSafeCast` fast path (unchanged), a `nullOnOverflow` branch (inline), and the ANSI throw branch which now emits a one-line `CastUtils.changePrecisionExact(...)` call instead of the 5-line `if/else` overflow block. ### Why are the changes needed? Part of SPARK-56908 (umbrella). The ANSI throw branch of `Cast.changePrecision` is hit by every cast to decimal that may overflow (very common in TPC-DS, where `cast(int as decimal(7,2))` is widespread). Collapsing the 5-line inline body to one line shrinks the generated Java source for those plans. ### Does this PR introduce _any_ user-facing change? No. The compiled behavior is identical; only the emitted Java source text changes. ### How was this patch tested? ``` build/sbt "catalyst/testOnly *CastSuite *CastWithAnsiOnSuite *CastWithAnsiOffSuite *AnsiCastSuite *TryCastSuite *DecimalSuite" ``` 332/332 pass. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Cursor 1.x Closes #55936 from gengliangwang/SPARK-56911-cast-decimal. Authored-by: Gengliang Wang <gengliang@apache.org> Signed-off-by: Gengliang Wang <gengliang@apache.org> (cherry picked from commit 8af1b8b) Signed-off-by: Gengliang Wang <gengliang@apache.org>

gengliangwang mentioned this pull request May 17, 2026

[SPARK-56909][SQL] Simplify Cast to int/long codegen under ANSI mode #55934

Closed

gengliangwang force-pushed the SPARK-56911-cast-decimal branch 2 times, most recently from 0beac87 to f699004 Compare May 17, 2026 23:35

gengliangwang requested review from cloud-fan and viirya May 17, 2026 23:39

gengliangwang marked this pull request as draft May 17, 2026 23:39

gengliangwang force-pushed the SPARK-56911-cast-decimal branch from f699004 to 8f72067 Compare May 18, 2026 17:07

gengliangwang marked this pull request as ready for review May 22, 2026 22:24

gengliangwang force-pushed the SPARK-56911-cast-decimal branch from 8f72067 to 62ce0cd Compare May 22, 2026 22:24

gengliangwang mentioned this pull request May 23, 2026

[SPARK-57004][SQL] Simplify CheckOverflowInSum codegen under ANSI mode #56063

Closed

gengliangwang force-pushed the SPARK-56911-cast-decimal branch from 62ce0cd to fd9228c Compare May 27, 2026 18:48

remove follow-up-PRs note from CastUtils.changePrecision javadoc

800fe2f

cloud-fan reviewed May 27, 2026

View reviewed changes

address review nits: widen CastUtils class javadoc, document changePr…

edf8784

…ecisionExact invariant, drop redundant getContextOrNullCode arg

viirya approved these changes May 27, 2026

View reviewed changes

gengliangwang closed this in 8af1b8b May 28, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-56911][SQL] Simplify Cast to decimal codegen under ANSI mode#55936

[SPARK-56911][SQL] Simplify Cast to decimal codegen under ANSI mode#55936
gengliangwang wants to merge 3 commits into
apache:masterfrom
gengliangwang:SPARK-56911-cast-decimal

gengliangwang commented May 17, 2026 •

edited

Loading

Uh oh!

gengliangwang commented May 17, 2026 •

edited

Loading

Uh oh!

gengliangwang commented May 21, 2026

Uh oh!

cloud-fan left a comment

Uh oh!

cloud-fan May 27, 2026

Uh oh!

gengliangwang May 27, 2026

Uh oh!

cloud-fan May 27, 2026

Uh oh!

gengliangwang May 27, 2026

Uh oh!

cloud-fan May 27, 2026

Uh oh!

gengliangwang May 27, 2026

Uh oh!

viirya commented May 27, 2026

Uh oh!

gengliangwang commented May 27, 2026

Uh oh!

gengliangwang commented May 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// Mutates the input Decimal in place to apply the target precision/scale on
		// the per-row hot path.

	val errorContextCode = getContextOrNullCode(ctx, !nullOnOverflow)
	val errorContextCode = getContextOrNullCode(ctx)

Conversation

gengliangwang commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

gengliangwang commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Stack overview (SPARK-56908 umbrella)

Uh oh!

gengliangwang commented May 21, 2026

Uh oh!

cloud-fan left a comment

Choose a reason for hiding this comment

Uh oh!

cloud-fan May 27, 2026

Choose a reason for hiding this comment

Uh oh!

gengliangwang May 27, 2026

Choose a reason for hiding this comment

Uh oh!

cloud-fan May 27, 2026

Choose a reason for hiding this comment

Uh oh!

gengliangwang May 27, 2026

Choose a reason for hiding this comment

Uh oh!

cloud-fan May 27, 2026

Choose a reason for hiding this comment

Uh oh!

gengliangwang May 27, 2026

Choose a reason for hiding this comment

Uh oh!

viirya commented May 27, 2026

Uh oh!

gengliangwang commented May 27, 2026

Uh oh!

gengliangwang commented May 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gengliangwang commented May 17, 2026 •

edited

Loading

gengliangwang commented May 17, 2026 •

edited

Loading